Repository navigation
fix(agc): reuse a recycled descriptor set only for a table of its own size - #6567
Conversation
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
Demon's Souls (PPSA01341) reaches this. On I ran it with a small LD_PRELOAD allocator that ends every 0x1f0..0x210-byte block at a PROT_NONE page. It runs slower that way, and both runs on With this PR on the same |
|
Thanks @fma-rc, that's the first title reported to reach this, and the guard-page runs pin it to the same write. I added your result to Tested. AI-assisted: yes (Claude Code) |
|
A Windows data point to go with fma-rc's: Demon's Souls (PPSA01342), RTX 3060, NVIDIA 591.86, Windows Server 2025 (26100), MinGW-w64 GCC 15.2.0, Release. The per-run table is on #6537. Every run with the fix had both this PR and #6537 cherry-picked (patch-identical to 544e7af and 9800f3a), so these runs can't say which one matters for this title.
AI-assisted: yes |
|
Thanks @fdfawef324, a second platform and driver on Demon's Souls: with #6537 and this PR, 2 of 4 runs reached your 3000 s limit, where all 6 without died. I added your result to Tested, with your note that these runs can't tell the two apart. AI-assisted: yes (Claude Code) |
|
Death Stranding 2 (PPSA02015) reaches this too, Linux, RTX 5080. On my DS2 stack without this PR, runs died on glibc heap errors ( The test also holds on current main 55b968a, RADV on a 680M: main plus only your test hunk fails 3/3 with |
|
Thanks @fma-rc. Death Stranding 2 is the second game reported to reach this, after Demon's Souls, and the RADV runs show that the test holds on current AI-assisted: yes (Claude Code) |
|
Demon's Souls (PPSA01341 v1.07) on Linux hits this too. Here the base has neither #6537 nor this PR, so the runs separate the two causes. Setup: Nobara 44 (kernel 7.2.0), RTX 5070 Ti, NVIDIA 595.91.07, GCC 16.2.1, Release. The base is Without the change, 5 of 5 runs died in the level between 366 s and 680 s on glibc heap checks: With this PR's commit 544e7af cherry-picked onto that base, and only Your AI-assisted: yes (Claude Code) |
|
Thanks @fisherjoey, I've added your runs to Tested. Like fma-rc's earlier Demon's Souls runs, yours are on a base without #6537, so the fix arm has this change and not #6537's. The Windows runs had both and couldn't tell them apart. AI-assisted: yes (Claude Code) |
|
#6454 has landed on Here is how we resolved it on our local stack, keeping
With that on AI-assisted: yes |
… size DescriptorCache::AllocateRewritten handed out any recycled set of the requested layout. The variable descriptor count of an image-table binding is fixed when its set is allocated, and PrepareDrawBindings writes the table's present elements, up to its current element count, into the set it gets. A draw whose table had more elements than the table the recycled set was allocated for could write descriptors past the set's allocation (VUID-VkWriteDescriptorSet-dstArrayElement-00321). A set from a chain pool now records the variable count it was allocated with, as a dedicated-pool set already does, and recycled sets are kept per layout and variable count, so a rewritten draw reuses a set only for a table of the same size. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
544e7af to
04aef89
Compare
|
@fisherjoey thanks. The rebase is pushed as 04aef89 on
On a Linux container with lavapipe, AI-assisted: yes (Claude Code) |
|
Confirming this on a title: Marvel's Wolverine (PPSA03671), Linux, NVIDIA GeForce RTX 4070 SUPER (driver 615.71.09), on a tree with #7395. Before this change the game died at about present 580, during the first build of an image table with 12,447 entries. The host heap was corrupted, and the crash site changed from run to run. With I found it independently and made the same change locally: the recycled sets are keyed by layout and variable count. With it, 6 of 6 runs of 240 seconds survive. Before, 6 of 6 crashed at about 110 seconds. So this PR fixes the crash that stops Wolverine there. |
What
DescriptorCache::AllocateRewrittenhanded out any recycled set of the requested layout.PrepareDrawBindingstakes that path for the snapshot sets of recorded draws (every draw unlessAPS5_SYNC_DRAWSis set).PrepareDrawBindingswrites the table's present elements, up to its current element count, into whichever recycled set it gets.VUID-VkWriteDescriptorSet-dstArrayElement-00321.A set from a chain pool now records the variable count it was allocated with, as a dedicated-pool set already does since #6454. Recycled sets are kept per layout and variable count, so a draw reuses a set only for a table of the same size, and allocates otherwise.
Tested
Linux 7.2.10 (CachyOS), RTX 3080 with NVIDIA 615.78.08, GCC 16.2.1, Release,
BUILD_TESTING=ON.main8a526d4, after perf(agc): share one image table per heap and write epoch through capture, compare, resolution and descriptors #6454 merged. Arch Linux container on a 16-thread Xeon E5-2640 v3, GCC 16.2.1, Release, lavapipe (Mesa 26.2.4).mainplus only the test hunk:agc_driver_graphicsandagc_descriptor_cachefail witha recycled descriptor set served a larger variable image table than it was allocated for.agc_descriptor_pool_reset_failureandagc_compute_descriptor_layout_flags.ctest -R 'agc|shader|recompiler|request|tess': this change 747 of 765 pass, andmain8a526d4 built the same way also 747 of 765. Both fail the same 18; the container does not permit userfaultfd, and 15 of them report guest memory that is not write-watched or needs to be.main.agc_driver_graphics(descriptorCacheTests, mock device) has a new case: a set allocated for a 3-element table is recycled, then the same layout is asked for with 5 elements and again with 3.mainfa9365c it fails witha recycled descriptor set served a larger variable image table than it was allocated for.ctest -R '^agc': 596 tests, 1 failure, 1 skipped.agc_driver_store_over_untrackedfails onmaintoo (test(agc): wait for the actual image in repeated untracked stores #6359, test(agc): keep waiting for the neighbour copy case's result #6385), andagc_driver_stencil_ref_exportis skipped.main4186f06 with local changes the title needs, plus fix(agc): keep each set's image table writes in its own draw snapshot… #6537's change, without this one;dstArrayElement-00321.00321messages 007 showed before came from fix(agc): keep each set's image table writes in its own draw snapshot… #6537's cause. This change rests on the test above.main5d9dcc4 (fix(agc): reuse a recycled descriptor set only for a table of its own size #6567 (comment)):mainit crashed on the driver thread in 4 of 6 runs once it reached character creation;mainfaulted in the same scene of the opening cinematic, on a store 16 bytes past a 0x208-byte block insidevkUpdateDescriptorSetsfromShaderResources::PrepareDrawBindings;agc_driver_graphicspasses on an RTX 5080 (NVIDIA 615.78.08), a Radeon 610M (RADV) and lavapipe.maina5e2a80 plus other PRs and a local change, listed at fix(agc): keep each set's image table writes in its own draw snapshot… #6537 (comment)):corrupted double-linked listat 470 s, andmunmap_chunk(): invalid pointerat 390 s withMALLOC_CHECK_;main55b968a with RADV on a Radeon 680M, run by fma-rc:mainplus only the test hunk fails 3 of 3 runs witha recycled descriptor set served a larger variable image table than it was allocated for. With the change it passes 3 of 3, andmainalone, without the test hunk, passes 3 of 3.main93e54ce, from before fix(agc): keep each set's image table writes in its own draw snapshot… #6537, plus fix(agc): keep texture images inside their extent's mip chain #6280's driver commit b9badf4 and a local stencil patch that doesn't touch descriptors; fix(agc): reuse a recycled descriptor set only for a table of its own size #6567 (comment)):libSceAgcDriver.prx, the run reached the 900 s stop with no heap message, core or device loss. Their own patch with the sameShaderResources.hpp/.cpphunks, written before they found this PR, did the same in 3 of 3 runs;agc_descriptor_cache,agc_driver_graphics,agc_descriptor_pool_reset_failureandagc_compute_descriptor_layout_flagspass on lavapipe with 544e7af onmainccb0c09.Related
PrepareDrawBindingspaired the fragment image table's write with the set-0 binding.std::optional<std::uint32_t> variableCounttoSetAllocationfor its kept sets and set it on dedicated-pool sets. This change keeps that member, fills it onAllocate's two chain-pool returns (a hit in an existing chained pool, and a new chained pool), and keysrecycledby layout and count. Pooled sets keep a null layout and no sizes, as onmain.Checklist
main; no other open PR implements the same functions.mdfiles or images added to the repository🤖 Generated with Claude Code